feat: add debug_getRawReceipts method#4919
feat: add debug_getRawReceipts method#4919BartoszSolkaBD wants to merge 18 commits intohiero-ledger:mainfrom
debug_getRawReceipts method#4919Conversation
Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
…ero-ledger#4889) Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
…r#4889) Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
…4889) Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
Signed-off-by: BartoszSolkaBD <bartosz.solka@blockydevs.com>
There was a problem hiding this comment.
LG!
The algorithm appears to match my understanding from the Yellow Paper (and the comments align as well). However, it’s all a little bit complicated for me, that's why I proposed this: https://github.com/hiero-ledger/hiero-json-rpc-relay/pull/4919/changes#r2804276011 .
packages/server/tests/acceptance/data/conformity/overwrites/debug_getRawReceipts/get-block-n.io
Outdated
Show resolved
Hide resolved
…eipts and eth_getBlockReceipts (hiero-ledger#4892) Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
…larity (hiero-ledger#4892) Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
jasuwienas
left a comment
jasuwienas
left a comment
There was a problem hiding this comment.
LG! The data we used in one of the unit test as input is the same as:
curl -sS "https://docs-demo.monad-mainnet.quiknode.pro/" -H 'Content-Type: application/json' --data '{"jsonrpc":"2.0","id":1,"method":"eth_getRawReceipts","params":["0x23592c0"]}'and we get the expect result the same as:
curl -sS "https://docs-demo.monad-mainnet.quiknode.pro/" -H 'Content-Type: application/json' --data '{"jsonrpc":"2.0","id":1,"method":"debug_getRawReceipts","params":["0x23592c0"]}'It’s not directly related to this change, but debug.spec.ts has grown quite large. We could make it much smaller by moving the test cases into separate file(s).
Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
…ementation (hiero-ledger#4892) Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
…penrpc-json-updated (hiero-ledger#4892) Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
…Receipts-method Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
| }); | ||
| }); | ||
|
|
||
| describe('debug_getRawReceipts', () => { |
There was a problem hiding this comment.
Is it worth adding more advanced checks like these here https://github.com/hiero-ledger/hiero-json-rpc-relay/pull/4909/changes#diff-ce8ccd45e52cf90b1bd467b689269510421319c309b4e08a8232707ebe3aa63bR1005?
I think the most beneficial tests are e2e, and from the beginning, we're trying to make them as precise as we can.
There was a problem hiding this comment.
we have a test here https://github.com/hiero-ledger/hiero-json-rpc-relay/pull/4919/changes#diff-a0d7caed18547ec84f2f466ff3e64154ff1eae3f7d6c6f8ae25c9a3153f5e52cR2030 that validates the fields individually. Please let me know if you think this is sufficient, or if we should also add similar checks to the acceptance tests.
…ceipts functionality (hiero-ledger#4892) Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
…ger#4892) Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
packages/relay/src/lib/services/ethService/blockService/blockWorker.ts
Outdated
Show resolved
Hide resolved
packages/relay/src/lib/services/ethService/blockService/blockWorker.ts
Outdated
Show resolved
Hide resolved
packages/relay/src/lib/services/ethService/blockService/blockWorker.ts
Outdated
Show resolved
Hide resolved
packages/relay/src/lib/services/ethService/blockService/blockWorker.ts
Outdated
Show resolved
Hide resolved
…Receipts-method Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
…ionReceiptFactory (hiero-ledger#4892) Signed-off-by: Bartosz Solka <bartosz.solka@blockydevs.com>
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #4919 +/- ##
==========================================
- Coverage 95.86% 92.01% -3.86%
==========================================
Files 143 115 -28
Lines 23968 19924 -4044
Branches 1900 1474 -426
==========================================
- Hits 22977 18333 -4644
- Misses 967 1525 +558
- Partials 24 66 +42
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 69 files with indirect coverage changes 🚀 New features to boost your workflow:
|
quiet-node
left a comment
quiet-node
left a comment
There was a problem hiding this comment.
Looking better some last items
| const encodedReceiptPromises = contractResults.map(async (contractResult) => { | ||
| if (Utils.isRejectedDueToHederaSpecificValidation(contractResult)) { | ||
| logger.debug( | ||
| `Transaction with hash %s is skipped due to hedera-specific validation failure (%s)`, | ||
| contractResult.hash, | ||
| contractResult.result, | ||
| ); | ||
| return null; | ||
| } | ||
|
|
||
| contractResult.logs = logsByHash.get(contractResult.hash) || []; | ||
|
|
||
| const receiptRlpInput = createReceiptRlpInput(contractResult.logs, contractResult, blockGasUsedBeforeTransaction); | ||
| blockGasUsedBeforeTransaction += contractResult.gas_used; | ||
| return encodeReceiptToHex(receiptRlpInput); | ||
| }); | ||
|
|
||
| const resolvedEncodedReceipts = await Promise.all(encodedReceiptPromises); | ||
| encodedReceipts.push( |
There was a problem hiding this comment.
Looks like the encodedReceiptPromises implementation has a misleading pattern. The callbacks in this method are marked as async but contain no await, meaning no actual asynchronous work is being done. Wrapping them in Promise.all provides no concurrency benefit since there is no I/O happening. We should replace this with a regular synchronous loop.
| contractResult.logs = logsByHash.get(contractResult.hash) || []; | ||
|
|
||
| const receiptRlpInput = createReceiptRlpInput(contractResult.logs, contractResult, blockGasUsedBeforeTransaction); | ||
| blockGasUsedBeforeTransaction += contractResult.gas_used; |
There was a problem hiding this comment.
This code is directly mutating contractResult.logs on a shared input object. Since contractResults is returned by getContractResults and referenced in a subsequently created Set and other local structures, mutating it inside an encoding helper is an unintended side effect and not a good practice.
Instead, we should pass the logs explicitly without patching the object:
| contractResult.logs = logsByHash.get(contractResult.hash) || []; | |
| const receiptRlpInput = createReceiptRlpInput(contractResult.logs, contractResult, blockGasUsedBeforeTransaction); | |
| blockGasUsedBeforeTransaction += contractResult.gas_used; | |
| const logs = logsByHash.get(contractResult.hash) || []; | |
| const receiptRlpInput = createReceiptRlpInput(logs, contractResult, blockGasUsedBeforeTransaction); |
or
| contractResult.logs = logsByHash.get(contractResult.hash) || []; | |
| const receiptRlpInput = createReceiptRlpInput(contractResult.logs, contractResult, blockGasUsedBeforeTransaction); | |
| blockGasUsedBeforeTransaction += contractResult.gas_used; | |
| const receiptRlpInput = createReceiptRlpInput(logsByHash.get(contractResult.hash) || [], contractResult, blockGasUsedBeforeTransaction); |
Also worth confirming: contractResult.logs is not actually used inside createReceiptRlpInput(), right?
| /** | ||
| * Creates a minimal receipt payload for RLP-encoding of a regular transaction. | ||
| * | ||
| * Builds an `IReceiptRlpInput` from mirror node contract result data and the | ||
| * running cumulative gas used before this transaction. The returned shape | ||
| * contains only the fields required for Yellow Paper receipt encoding, including the updated cumulative gas used, | ||
| * logs and bloom, root and status, transaction index, and normalized type. | ||
| * @param params - Parameters required to build the RLP input, including | ||
| * contract result data, associated logs, and the cumulative gas used prior | ||
| * to this transaction. | ||
| * @returns Minimal receipt data suitable for RLP encoding. | ||
| */ | ||
| function createReceiptRlpInput( | ||
| logs: Log[], | ||
| receiptResponse: any, | ||
| blockGasUsedBeforeTransaction: number, | ||
| ): IReceiptRlpInput { | ||
| return { | ||
| cumulativeGasUsed: numberTo0x(blockGasUsedBeforeTransaction + receiptResponse.gas_used), | ||
| logs: logs, | ||
| logsBloom: receiptResponse.bloom === constants.EMPTY_HEX ? constants.EMPTY_BLOOM : receiptResponse.bloom, | ||
| root: receiptResponse.root || constants.DEFAULT_ROOT_HASH, | ||
| status: receiptResponse.status, | ||
| transactionIndex: numberTo0x(receiptResponse.transaction_index), | ||
| type: nanOrNumberTo0x(receiptResponse.type), | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Creates a minimal receipt payload for RLP-encoding of a synthetic transaction. | ||
| * | ||
| * Builds an `IReceiptRlpInput` from synthetic logs only, without resolving any | ||
| * addresses or constructing a full `ITransactionReceipt`. The returned shape | ||
| * contains the fields required for Yellow Paper receipt encoding, including a zero | ||
| * cumulative gas used, zero gas used, a logs bloom computed from the first | ||
| * synthetic log, default root and status values, the transaction index from | ||
| * the first log, and a fallback type of `0x0`. | ||
| * | ||
| * @param syntheticLogs - Logs belonging to the synthetic transaction. | ||
| * @returns Minimal receipt data suitable for RLP encoding. | ||
| */ | ||
| function createSyntheticReceiptRlpInput(syntheticLogs: Log[]): IReceiptRlpInput { | ||
| return { | ||
| cumulativeGasUsed: constants.ZERO_HEX, | ||
| logs: syntheticLogs, | ||
| logsBloom: LogsBloomUtils.buildLogsBloom(syntheticLogs[0].address, syntheticLogs[0].topics), | ||
| root: constants.DEFAULT_ROOT_HASH, | ||
| status: constants.ONE_HEX, | ||
| transactionIndex: syntheticLogs[0].transactionIndex, | ||
| type: constants.ZERO_HEX, // fallback to 0x0 from HAPI transactions | ||
| }; | ||
| } | ||
|
|
There was a problem hiding this comment.
Can these two be unified something like
function createReceiptRlpInput(
logs: Log[],
receiptResponse?: any,
blockGasUsedBeforeTransaction?: number,
isSynthetic: boolean
): IReceiptRlpInput {
return {
cumulativeGasUsed: isSynthetic ? ...
logs,
logsBloom: isSynthetic ? ...
root: isSynthetic ? ...
status: isSynthetic ? ...
transactionIndex: isSynthetic ? ...
type: isSynthetic ? ...
};
}Or should we keep it separate for readibility?
There was a problem hiding this comment.
Strange I really thought I left a comment on this file in the previous round of review. Anyhoo should we consider moving this module out of the root of src/lib where the other generic namespace modules live? It feels like the receipt-related logic here would be better encapsulated in a dedicated class rather than sitting alongside the other top-level modules. Since these methods are all doing transaction receipt related work, would it make sense to move them into the TransactionReceiptFactory class? That seems like a natural home for them and would keep things cleaner overall.
5 participants
Description
This PR adds support for the debug_getRawReceipts JSON-RPC method. It returns an array of EIP-2718 binary-encoded transaction receipts for a given block.
Changes:
Related issue(s)
Fixes #4892
Testing Guide
Start the JSON-RPC relay with mainnet config (and DEBUG_API_ENABLED=true if required). Ensure it is connected and serving requests.
{"jsonrpc":"2.0","id":1,"method":"debug_getRawReceipts","params":["0x56e86ab"]}{"jsonrpc":"2.0","id":1,"method":"eth_getBlockReceipts","params":["0x56e86ab"]}Changes from original design (optional)
N/A
Additional work needed (optional)
N/A
Checklist